Skip to content

Conversation

@DaveCTurner
Copy link
Contributor

Today :server:systemPropertiesOverrideTest uses the same source set as
:server:test, but runs a disjoint set of tests via includes &
excludes. This forces every developer to choose in their IDE between the
two tasks when running any unit test in :server even though there's
only one appropriate task to run for any particular test.

This commit moves the serverless-specific unit tests into their own
source set to remove this friction in the development flow.

Today `:server:systemPropertiesOverrideTest` uses the same source set as
`:server:test`, but runs a disjoint set of tests via includes &
excludes. This forces every developer to choose in their IDE between the
two tasks when running any unit test in `:server` even though there's
only one appropriate task to run for any particular test.

This commit moves the serverless-specific unit tests into their own
source set to remove this friction in the development flow.
@DaveCTurner DaveCTurner requested a review from ywangd November 2, 2025 13:44
@DaveCTurner DaveCTurner added >test Issues or PRs that are addressing/adding tests :Distributed Coordination/Distributed A catch all label for anything in the Distributed Coordination area. Please avoid if you can. v9.3.0 labels Nov 2, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)

@elasticsearchmachine elasticsearchmachine added the Team:Distributed Coordination Meta label for Distributed Coordination team label Nov 2, 2025
Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks for improving this!


tasks.named("test").configure {
systemProperty 'es.stateless.allow.index.refresh_interval.override', 'true'
// systemProperty 'es.serverless_transport', 'true'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still need this, right? I wonder why the CI didn't fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh hmm that's a good point - I commented that out for a test but then forgot I'd done so. CI passes because we weakened this to a warning in #128589 but did not adjust the es.serverless_transport test to assert the lack of a warning. Fixed in 7c9ffc2.

@DaveCTurner DaveCTurner enabled auto-merge (squash) November 5, 2025 09:06
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Nov 5, 2025
These tests relate to system properties set only in serverless
Elasticsearch so there's no need for them in the `8.19` branch.
Moreover, as with elastic#137492, this extra Gradle task forces every developer
to make an extra choice when running tests in `:server`. This commit
removes the unnecessary tests and the corresponding Gradle task.
@DaveCTurner DaveCTurner merged commit 7515dfd into elastic:main Nov 5, 2025
34 checks passed
DaveCTurner added a commit that referenced this pull request Nov 5, 2025
These tests relate to system properties set only in serverless
Elasticsearch so there's no need for them in the `8.19` branch.
Moreover, as with #137492, this extra Gradle task forces every developer
to make an extra choice when running tests in `:server`. This commit
removes the unnecessary tests, the corresponding Gradle task, and the
now-untested code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Coordination/Distributed A catch all label for anything in the Distributed Coordination area. Please avoid if you can. Team:Distributed Coordination Meta label for Distributed Coordination team >test Issues or PRs that are addressing/adding tests v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants